Screenshot path pattern (closes #1602, #1651, #1974, #1975) #2562
Conversation
miherlosev
force-pushed
the
screenshot_path_pattern
branch
from
c0bd28d
to
ec61e6f
Compare
|
✅ Tests for the commit 07ab649 have passed. See details: |
miherlosev
changed the title
src/cli/argument-parser.js
Outdated
| @@ -18,8 +17,8 @@ const REMOTE_ALIAS_RE = /^remote(?::(\d*))?$/; | |||
| const DEFAULT_TEST_LOOKUP_DIRS = ['test/', 'tests/']; | |||
| const TEST_FILE_GLOB_PATTERN = `./**/*@(${Compiler.getSupportedTestFileExtensions().join('|')})`; | |||
|
|
|||
| const DESCRIPTION = dedent(` | |||
| In the browser list, you can use browser names (e.g. "ie9", "chrome", etc.) as well as paths to executables. | |||
| const DESCRIPTION = ` | |||
There was a problem hiding this comment.
Yes, need to revert back this change
|
❌ Tests for the commit b81b460 have failed. See details: |
|
@testcafe-build-bot retest |
|
❌ Tests for the commit b81b460 have failed. See details: |
|
@testcafe-build-bot retest |
|
❌ Tests for the commit b81b460 have failed. See details: |
|
@testcafe-build-bot retest |
|
❌ Tests for the commit b81b460 have failed. See details: |
|
@testcafe-build-bot retest |
|
❌ Tests for the commit b81b460 have failed. See details: |
|
@testcafe-build-bot retest |
|
✅ Tests for the commit b81b460 have passed. See details: |
|
FPR |
|
@VasilyStrelyaev, please check the new option description: https://github.com/DevExpress/testcafe/pull/2562/files#diff-032d695d390a3643d7f0e829fdb8097cR91 |
src/runner/index.js
Outdated
| @@ -198,9 +197,10 @@ export default class Runner extends EventEmitter { | |||
| return this; | |||
| } | |||
|
|
|||
| screenshots (path, takeOnFails = false) { | |||
| screenshots (path, takeOnFails = false, pattern) { | |||
There was a problem hiding this comment.
Please, add a default value to make clear that pattern is optional.
src/cli/argument-parser.js
Outdated
| @@ -89,6 +88,7 @@ export default class CLIArgumentParser { | |||
| .option('-r, --reporter <name[:outputFile][,...]>', 'specify the reporters and optionally files where reports are saved') | |||
| .option('-s, --screenshots <path>', 'enable screenshot capturing and specify the path to save the screenshots to') | |||
| .option('-S, --screenshots-on-fails', 'take a screenshot whenever a test fails') | |||
| .option('-p, --screenshot-path-pattern <pattern>', 'use patterns to compose screenshot file names and paths: ${BROWSER}, ${BROWSER_VERSION}, ${OS}, ${OS_VERSION}, ${USERAGENT}, ${DATE}, ${TIME}, ${FIXTURE}, ${TEST}, ${TEST_INDEX}, ${FILE_INDEX}, ${QUARANTINE_ATTEMPT}') | |||
There was a problem hiding this comment.
Maybe make it a little shorter?
.option('-p, --screenshot-path-pattern <pattern>', 'use patterns to compose screenshot file names and paths: ${BROWSER}, ${BROWSER_VERSION}, ${OS}, etc.')
Users will have to read documentation to use this feature anyway. So there's no need to list here all the variables. We can do it in the docs,
There was a problem hiding this comment.
Agree. Without full description for some placeholders (${FILE_INDEX}, ${QUARANTINE_ATTEMPT}) it's a difficult to guess the replaced value.
|
❌ Tests for the commit b2c9875 have failed. See details: |
|
@testcafe-build-bot retest |
|
❌ Tests for the commit b2c9875 have failed. See details: |
|
@testcafe-build-bot retest |
|
✅ Tests for the commit b2c9875 have passed. See details: |
|
FPR @churkin |
| static _getScreenshotBaseDirName () { | ||
| var now = Date.now(); | ||
|
|
||
| return moment(now).format('YYYY-MM-DD_hh-mm-ss'); |
There was a problem hiding this comment.
@miherlosev You also fixed another bug here: the hours were not in 24h format, but are now! 🎉🎉🎉 Thank you!
…Express#1974, DevExpress#1975) (DevExpress#2562) * Screenshot file pattern * refactor test constrants * small refactoring * renames * don't correct file path for custom file path * try to fix tests2 * revert back correct file path for custom path * revert dedent module * fix review issues
No description provided.